Skip to content

[Ellen's Alien Game] Test that an instance variable is used for the health, not a class variable. - #4273

Merged
BethanyG merged 5 commits into
exercism:mainfrom
IsaacG:instance_var
Aug 10, 2026
Merged

[Ellen's Alien Game] Test that an instance variable is used for the health, not a class variable.#4273
BethanyG merged 5 commits into
exercism:mainfrom
IsaacG:instance_var

Conversation

@IsaacG

@IsaacG IsaacG commented Jul 28, 2026

Copy link
Copy Markdown
Member

In response to this forum post.

@github-actions

Copy link
Copy Markdown
Contributor

This PR touches files which potentially affect the outcome of the tests of an exercise. This will cause all students' solutions to affected exercises to be re-tested.

If this PR does not affect the result of the test (or, for example, adds an edge case that is not worth rerunning all tests for), please add the following to the merge-commit message which will stops student's tests from re-running. Please copy-paste to avoid typos.

[no important files changed]

For more information, refer to the documentation. If you are unsure whether to add the message or not, please ping @exercism/maintainers-admin in a comment. Thank you!

@IsaacG

IsaacG commented Jul 28, 2026

Copy link
Copy Markdown
Member Author

This solution uses a class var for the health. It passes the existing tests and fails this new test.

@BethanyG BethanyG left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one nit (and the error was in the test file already). Either a space or a \n will do.

Per our Discord discussion, I'd love it if we added some comments and also cleaned up that one test. I'll put a comment below, once I get out of review mode.

PS - comments don't have to be long or exhaustive, but maybe some ... breadcrumbs so we don't forget what happened......

# pylint: disable=raise-missing-from
raise ImportError("\n\nMISSING CLASS --> We tried to import the 'Alien' class from "
"your classes.py file, but could not find it."
"your classes.py file, but could not find it."

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The space here needs to remain (but inside the quotes, or a \n could be used as well.) Otherwise, with Python's string concatenation rules, it will be smoosed together with the following sentence with no space at all between. And indeed: its broken in production right now. Another alternative is preceding the follow-on sentence.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

# pylint: disable=raise-missing-from
raise ImportError("\n\nMISSING CLASS --> We tried to import the 'Alien' class from "
"your classes.py file, but could not find it."
"your classes.py file, but could not find it."

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"your classes.py file, but could not find it."
"your classes.py file, but could not find it. "

@BethanyG

Copy link
Copy Markdown
Member

The offending test, and my proposed edit (delete everything that's commented out)

    @pytest.mark.task(taskno=6)
    def test_alien_class_variable(self):
        """Test class attribute/variables are identical across instances."""

        alien_one, alien_two = Alien(0, 2), Alien(-6, -1)
        # Alien.health = 6

        created_error_message = ('Created two new Aliens and requested the '
                                 'total_aliens_created attribute for each one. '
                                 f'Received {alien_one.total_aliens_created, alien_two.total_aliens_created} '
                                 f'for total_aliens_created, but the tests expect '
                                 f'the class attributes for each newly created Alien to be identical. ')

        # health_error_message = ('Created two new Aliens and requested the '
        #                         f'health attribute for each one. Received {alien_one.health, alien_two.health} '
        #                         'for health, but the tests expect the class '
        #                         'attributes for each newly created Alien to be identical. ')

        self.assertEqual(alien_two.total_aliens_created,
                         alien_one.total_aliens_created,
                         msg=created_error_message)
        #
        # self.assertEqual(alien_two.__class__.health,
        #                  alien_one.__class__.health,
        #                  msg=health_error_message)

@BethanyG BethanyG left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Thank you so much for doing this! 🚀

@BethanyG
BethanyG merged commit 1f6aab8 into exercism:main Aug 10, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants